Add unit testing#18
Conversation
|
Note that all changes to files in |
Akash-Maurya-0899
left a comment
There was a problem hiding this comment.
Hi @prayush, I have added some comments. The major one is regarding the black formatting test, which may simply fail due to different black versions used by the tests vs by a developer on their local machine. The fix would be to pin a particular version black for the tests, and also add it as an optional "developer" dependency of esigmapy. More details in the comments.
| # 1.5 lies between arr[1]=1 and arr[2]=2; nearest is equidistant → either valid | ||
| arr = np.array([0.0, 1.0, 2.0, 3.0]) | ||
| idx = find_first_value_location_in_series(arr, 1.5) | ||
| assert idx in (1, 2) |
There was a problem hiding this comment.
Shouldn't it check if the output is 1, and not 1 or 2 (because that's what find_first_value_location_in_series would output I think)? Or is it that due to floating point rounding error, it can be either 1 or 2?
There was a problem hiding this comment.
yes, find_first_value_location_in_series is finding the first location in the first input (array) which has its value closest to the requested value. So in this case both are valid. But your point is right, i.e. in the very special case that the desired value is right in the middle of two known values, what do we do? I have updated both the find_first_value_location_in_series function and its test here to now really return the "first" value location in such cases.
|
|
||
| - name: Install dependencies | ||
| run: | | ||
| pip install lalsuite pycbc pytest black |
There was a problem hiding this comment.
| # |3 + 4j| == 5 | ||
| wave = np.full(10, 3.0 + 4.0j) | ||
| amp = compute_amplitude(wave) | ||
| assert np.allclose(amp, 5.0) |
There was a problem hiding this comment.
Perhaps decrease the tolerances rtol and atol in np.allclose everywhere to make the tests more stringent.
153ef57 to
9db62e2
Compare
|
@Akash-Maurya-0899 all your comments have been addressed. If satisfied, approve this but please don't merge it yet. |
9db62e2 to
9603f74
Compare
and mocked coverage for inspiral and merger-ringdown generators - Add unit tests for inspiral and MR mode generation utilities using mocked lalsimulation / delegated generators. - Add deterministic unit tests for waveform utilities, blending helpers, and legacy fitting functions without using mocks. utils.py * verify f22/x roundtrip consistency across x and mass ranges * verify f22 scales as M^-1 * verify Schwarzschild ISCO frequency for zero spins * verify prograde spins increase ISCO frequency * verify retrograde spins decrease ISCO frequency * verify monotonic inputs produce no detected peaks * verify sinusoidal inputs return expected peak locations * verify equatorial-plane polarization reconstruction for (2,2) mode * verify face-on symmetry gives hx = 0 * verify polarization linearity under mode scaling blend.py * verify first-value lookup on constant arrays * verify first/last index lookup on increasing arrays * verify first/last symmetry properties * verify amplitude extraction for known complex inputs * verify amplitude for real-valued inputs * verify phase unwrapping for linear complex phase * verify constant-phase inputs produce flat phase * verify frequency recovery from linear phase evolution * verify decreasing phase produces negative frequencies * verify mismatch is zero for identical waveforms * verify mismatch is one for orthogonal waveforms * verify mismatch remains within [0, 1] * verify blended series matches endpoints correctly * verify blended series smoothness across transition region * verify blended series output length * verify phase alignment reduces mismatch * verify inverse alignments cancel net phase shift legacy.py * verify zero polynomial coefficients return unity * verify quadratic polynomial evaluation for known coefficients * verify equal numerator/denominator ratio polynomial returns unity * verify fitting functions remain positive for η ∈ (0, 0.25] * verify fitting functions are finite at η = 0.01 and η = 0.25 Included test coverage: * TestGetInspiralEsigmaModesWithMock (7 tests) - verify returned dict contains requested (l,m) key - verify include_conjugate_modes=True/False controls (l,-m) - verify default return type is pycbc.TimeSeries - verify return_pycbc_timeseries=False returns numpy arrays - verify distance conversion from Mpc to SI before C call - verify mode generation called once per unique mode * TestEccentricityAtReferenceFrequencyWithMock (2 tests) - verify nearest-index lookup logic for x_ref - verify returned eccentricity matches argmin|x - x_ref| * TestGenerateLalsimModesWithMock (5 tests) - verify missing approximant propagates AttributeError -> IOError - verify linked-list traversal filters requested modes only - verify all requested modes are returned - verify unrequested modes are ignored - verify extracted mode data is returned as numpy arrays * TestGetMrModesWithMock (4 tests) - verify f_ref=None defaults to f_lower - verify coa_phase=None defaults to 0.0 - verify NRSur7dq4 routes to _generate_lalsim_modes - verify SEOBNRv5HM routes to _generate_pyseobnr_modes - Enable unit tests to be run on github - Add unit test to ensure formatting with black
9603f74 to
5f25824
Compare
|
@Akash-Maurya-0899 this is now ready for "Rebase and Merge", if you are happy with the resolution of your comments. I have incorporated all changes you asked for. |
This PR adds substantial new unit test coverage for waveform generation, waveform utilities, blending helpers, and legacy fitting functions.
Generator and mode-generation tests
New mocked tests were added for inspiral and merger-ringdown mode generation using patched
lalsimulationand delegated generators. These tests verify:(l,m)modes,pycbc.TimeSeriesvs NumPy arrays),f_refandcoa_phase.Deterministic numerical tests
Additional pure-Python tests (without mocks) were added for
utils.py,blend.py, andlegacy.py.These tests verify:
Overall, this PR improves regression protection and numerical validation across the waveform generation and hybridization codebase while keeping the tests deterministic and lightweight for CI.